tests/unit/core/storage/test_sql_storage.py (1)

269-313: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add assertion for tag JSON serialization.

test_all_fields_stored sets tag=["integration"] but never asserts on row.tag, so the new json.dumps(result.tag) behavior in sql_storage.py isn't actually verified here (unlike expected_response, which has its own test_list_serialization). As per coding guidelines, "Add comprehensive pytest coverage for new features, including mocked LLM calls where applicable."

✅ Proposed assertion addition

         assert row is not None
         assert row.conversation_group_id == "conv_full"
         assert row.score == 0.92
         assert row.evaluation_latency == 2.5
         assert row.tool_calls == '[{"name": "search"}]'
+        assert row.tag == '["integration"]'

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/core/storage/test_sql_storage.py` around lines 269 - 313, The SQL
storage test covers full-field persistence but does not verify JSON
serialization for the tag field, so the new json.dumps(result.tag) behavior in
SQLStorageBackend.save_result is untested. Update test_all_fields_stored in
test_sql_storage.py to assert the persisted row.tag value matches the serialized
tag list, using the existing result setup and row fetched from
evaluation_results.

Source: Coding guidelines

tests/unit/core/output/test_statistics_detailed.py (1)

317-373: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing coverage for a single result with multiple tags.

All updated cases use single-element tag lists. None exercises the new multi-bucket behavior itself, i.e. a result whose tag contains more than one value (e.g., tag=["production", "staging"]) contributing to multiple by_tag groups simultaneously. Consider adding a case for that scenario to directly validate the for t in r.tag iteration added in compute_tag_stats.

✅ Example additional test case

def test_compute_detailed_stats_by_multiple_tags_single_result(self) -> None:
    """Test a single result with multiple tags contributes to each tag group."""
    results = [
        EvaluationResult(
            conversation_group_id="conv1",
            tag=["production", "staging"],
            turn_id="turn1",
            metric_identifier="metric1",
            result="PASS",
            score=0.9,
            threshold=0.7,
            reason="Good",
        ),
    ]

    stats = compute_detailed_stats(results).model_dump()

    assert "production" in stats["by_tag"]
    assert "staging" in stats["by_tag"]
    assert stats["by_tag"]["production"]["passed"] == 1
    assert stats["by_tag"]["staging"]["passed"] == 1

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/core/output/test_statistics_detailed.py` around lines 317 - 373,
The detailed stats tests still miss the new multi-tag bucket behavior. Add a
focused test alongside test_compute_detailed_stats_by_tag that uses a single
EvaluationResult with tag containing multiple values (for example, both
production and staging) and assert compute_detailed_stats()/compute_tag_stats
places that one result into each corresponding by_tag entry. Verify both tag
groups show the expected passed/failed counts so the for t in r.tag logic is
directly covered.

src/lightspeed_evaluation/core/models/data.py (1)

589-595: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid exposing validator helpers as public APIs without Google-style docs.

Either rename these validator methods to _normalize_tag or expand the docstrings to Google style. As per coding guidelines, src/lightspeed_evaluation/**/*.py: all public APIs must use Google-style docstrings.

Also applies to: 725-731

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lightspeed_evaluation/core/models/data.py` around lines 589 - 595, The
validator helper methods normalize_tag and the similar validator in the
referenced block are being exposed as public APIs without Google-style docs.
Rename these helpers to private names like _normalize_tag, or update their
docstrings to Google-style format so they comply with the project’s public API
documentation rules while keeping the field_validator behavior unchanged.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes